Skip to content

bhwi-cli: warn about network mismatch#29

Merged
edouardparis merged 2 commits intowizardsardine:mainfrom
trevarj:cli-network-warning
Apr 15, 2026
Merged

bhwi-cli: warn about network mismatch#29
edouardparis merged 2 commits intowizardsardine:mainfrom
trevarj:cli-network-warning

Conversation

@trevarj
Copy link
Copy Markdown
Collaborator

@trevarj trevarj commented Apr 10, 2026

Warn the user when the CLI's network config is inconsistent with the network
that is configured on the device.

Includes a refactor from get_version -> get_info

Fixes #23

@trevarj trevarj marked this pull request as draft April 10, 2026 18:56
@trevarj trevarj force-pushed the cli-network-warning branch from ce4c981 to aaa8e65 Compare April 10, 2026 19:04
@trevarj
Copy link
Copy Markdown
Collaborator Author

trevarj commented Apr 10, 2026

@edouardparis not sure about whether we want to have Version's network field be bitcoin::Network or not.
Just hard to check when they aren't all using a common type.

See:

impl From<JadeNetworks> for bitcoin::Network {
    fn from(networks: JadeNetworks) -> Self {
        match networks {
            JadeNetworks::Main | JadeNetworks::All => Self::Bitcoin,
            JadeNetworks::Test => Self::Testnet,
        }
    }
}

@trevarj trevarj requested a review from edouardparis April 12, 2026 10:35
@edouardparis
Copy link
Copy Markdown
Member

edouardparis commented Apr 13, 2026

I do not have a strong opinion about it. Most usage will be Mainnet/Bitcoin in the end.

We can change common::Response::Version (should be renamed I think for Info if this has multiple fields outside of version, and rename get_version for get_info), to have a networks [] field too.

Having info.networks.contains(network) as check instead of info.network == network is really decent for me.

trevarj added 2 commits April 14, 2026 13:24
This makes more sense since the struct actually contains more than just the
device firmware version.

Also change network field to networks for devices that may support multiple
networks at a time (currently just jade).
Show a warning to the user when the device's configured network doesn't match
what bhwi-cli expects.
@trevarj trevarj force-pushed the cli-network-warning branch from aaa8e65 to 4bafd7b Compare April 14, 2026 13:25
@trevarj
Copy link
Copy Markdown
Collaborator Author

trevarj commented Apr 14, 2026

I do not have a strong opinion about it. Most usage will be Mainnet/Bitcoin in the end.

We can change common::Response::Version (should be renamed I think for Info if this has multiple fields outside of version, and rename get_version for get_info), to have a networks [] field too.

Having info.networks.contains(network) as check instead of info.network == network is really decent for me.

@edouardparis this seems better. I force-pushed these changes. I think it's good enough for now and we can just rework later probably.

@trevarj trevarj marked this pull request as ready for review April 14, 2026 13:30
@edouardparis edouardparis merged commit c06aaf7 into wizardsardine:main Apr 15, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cli: descriptor pubkeys can maybe check if ledger is open with the testnet or mainnet app

2 participants